update reva and refactor config #657

butonic · 2020-10-06T14:36:17Z

this pr tries to clarify the storage provider setup and configuration for the ocis storage extension

update to latest reva
reduce driver specific config to /home, /user, /public storages and the metadata storage which is not accessible using the gateway
use ocis driver
requires ocis: allow setting owner of root cs3org/reva#1225, especially cs3org/reva@b93bc82 which fixes bugs that allow the accounts service to provision accounts when using the ocis driver
with ocis: allow setting owner of root cs3org/reva#1225 the accounts service can create /meta/accounts ... however the default users don't seem to be created in there, which causes subsequent logins to fail because the system users for konnectd and reva don't exist ... noone can login. AFAICT the accounts service needs to create the default accounts. fixed with cs3org/reva@b93bc82
for eos - and this is untested - the only thing that should be necessary is setting STORAGE_HOME_DRIVER=eoshome and STORAGE_USERS_DRIVER=eos. But since the docker compose starts all services at once we need to use ocis to stop and start the home and user storages:
```
docker-compose exec ocis ./bin/ocis kill storage-home
docker-compose exec -e STORAGE_HOME_DRIVER=eoshome ocis ./bin/ocis run storage-home
docker-compose exec ocis ./bin/ocis kill storage-users
docker-compose exec -e STORAGE_USERS_DRIVER=eos ocis ./bin/ocis run storage-users
```
The storages now start a dataprovider with the same driver, so an additional config for them is no longer necessary. Unfortunately, testing this is currently not possible because the reva user used by eos to access ldap ... does not exist when the new account service backend has not created the users ... this might also be related to ldap support not working.
🤔
I think there is a problem with the ldap setup step. https://github.com/owncloud-docker/eos-stack/blob/de26692b6d6544620b948fa8212a4fa57455ae1d/eos-ocis-dev/start-ldap#L10-L11 requires an LDAP_BINDDN ... which to be honest I have no clue how it is set because in all the owncloud orga we are not setting it for eos. even in owncloud-docker where https://github.com/owncloud-docker/compose-playground/blob/c563391876ed63b52ab2960df025947d4788a7fe/examples/eos-compose-acceptance-tests/docker-compose.yml#L91 looks most promising I assume it was intended for konnectd ...

but how did resolving users in the docker container with id einstein ever work ... 🤔 ... because we were not checking the credentials in glauth ... or accounts?? ... 🤪

IN any case I think we need to tell eos to use the LDAP_BINDDN and LDAP_BINDBW that match the accounts service user ... 95cb8724-03b2-11eb-a0a6-c33ef8ef53ad and we should probably configure it by default ... currently it is empty...

AFAICT this is good on its own but other wheels need fixing ...

update-docs · 2020-10-06T14:36:21Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

butonic · 2020-10-06T16:46:48Z

Interesting, the tests fail because of permissions errors:

2020-10-06T14:39:50Z ERR srv/app/pkg/mod/github.com/cs3org/reva@v1.2.2-0.20201006093611-4a9be347ac29/internal/grpc/services/storageprovider/storageprovider.go:399 > permission denied error="error: permission denied: root/accounts"

butonic · 2020-10-06T16:50:05Z

root/accounts is of the form nodeid:basename so the accounts dir is probably not owned by the user that tries to access the accounts folder in the root of the storage. It is configured to use the ocis driver...

butonic · 2020-10-06T16:50:48Z

log which user ID is trying to access the folder

butonic · 2020-10-06T17:06:25Z

The problem is that the ocis driver checks the permissions. While the user is provided by the accounts service the root:accounts node ... Has no owner... It has not even been created... So we need a way to create the folder... Or tell the ocis storage driver who owns the storage... Hmmm

butonic · 2020-10-06T17:09:16Z

Even if we add a config option for the owner... We still need to create the accounts dir. Or is the accounts service trying to create the folder if it does not exist? @IljaN @refs @kulmann

butonic · 2020-10-06T17:20:46Z

Cool, it does:

ocis/accounts/pkg/storage/cs3.go

Line 226 in f10950d

    
           func (r CS3Repo) makeRootDirIfNotExist(ctx context.Context, folder string) error {

butonic · 2020-10-06T17:35:37Z

We could add a config option for the owner. I would prefer that over having a storage that can be added by anyone.

But having a single user ID as the owner limits access to a single user... Which means we would have to share the credentials of that user with all services that want to use the metadata storage.

We could use a map with /path -> owner ID mapping

It could be used to set the owner if a node has no owner set. Which still does not work for root. The storage could precreate these paths when it is initialized and set the owner.

The map would be similar to the storage registry rules...

butonic · 2020-10-06T19:46:23Z

hm, the account service is also trying to create accounts. but it fails:

2020-10-06T21:43:37+02:00 INF access token is already provided pkg=rhttp service=storage traceid=a05d4064c45adf0501fc17224928ef23
2020-10-06T21:43:37+02:00 DBG http routing: head=data tail=/accounts/4c510ada-c86b-4815-8820-42cdf82c3d51 svc=data pkg=rhttp service=storage
2020-10-06T21:43:37+02:00 DBG NodeFromPath() fn=/accounts/4c510ada-c86b-4815-8820-42cdf82c3d51 pkg=rhttp service=storage traceid=a05d4064c45adf0501fc17224928ef23
2020-10-06T21:43:37+02:00 DBG NodeFromPath() walk node={"Exists":true,"ID":"69afd7ba-55f2-4678-9e4c-aeaf14c56aca","Name":"accounts","ParentID":"root"} pkg=rhttp service=storage traceid=a05d4064c45adf0501fc17224928ef23
2020-10-06T21:43:37+02:00 DBG NodeFromPath() walk node={"Exists":false,"ID":"","Name":"4c510ada-c86b-4815-8820-42cdf82c3d51","ParentID":"69afd7ba-55f2-4678-9e4c-aeaf14c56aca"} pkg=rhttp service=storage traceid=a05d4064c45adf0501fc17224928ef23
2020-10-06T21:43:37+02:00 ERR error uploading file error="error: permission denied: 69afd7ba-55f2-4678-9e4c-aeaf14c56aca/4c510ada-c86b-4815-8820-42cdf82c3d51" pkg=rhttp service=storage traceid=a05d4064c45adf0501fc17224928ef23

butonic · 2020-10-06T20:47:44Z

accounts/pkg/storage/cs3.go

@@ -206,12 +206,11 @@ func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) {

 func (r CS3Repo) authenticate(ctx context.Context) (token string, err error) {
 	u := &user.User{
-		Id:     &user.UserId{},
+		Id: &user.UserId{
+			OpaqueId: r.cfg.ServiceUser.UUID,


@kulmann we can always set the uuid ... if it is "" that is the same as this if magic ;-)

It is not empty by default. It has a default UUID value in the flatset, so that it just works as soon as the username is provided. You're right, but it needs more change than this. ;-) Clear the default value for the UUID in the flagset and replace the checks on the username flag with checks on the uuid flag.

yes it has a uuid, but before this change it would only set it if the username was also set ... but it is empty by default: https://github.com/owncloud/ocis/pull/657/files#diff-69d2327c70e84f0e2b037204de3ef8e3L212-L214

butonic · 2020-10-06T20:50:50Z

storage/pkg/config/config.go

 	// Protocol can be grpc or http
 	Protocol string
-	// URL is used by the gateway and registries (eg http://localhost:9100 or https://cloud.example.com)
-	URL string
+	// Endpoint is used by the gateway and registries (eg localhost:9100 or cloud.example.com)


this is the best I could come up with to distinguish addr, host, port and url ... an endpoint is a combination of a tcp host and a port ... try googling for it. the only two URLs are the public facing endpoints, which is why the are now called PublicURL

butonic · 2020-10-06T20:52:27Z

storage/pkg/flagset/frontend.go

@@ -42,7 +42,7 @@ func FrontendWithConfig(cfg *config.Config) []cli.Flag {
 		// this can eg. be set to /eos/users
 		&cli.StringFlag{
 			Name:        "dav-files-namespace",
-			Value:       "/oc/",
+			Value:       "/users/",


we no longer need to change this from /oc to eos because we onlyd change the driver of the users storage provider mounted at /users

butonic · 2020-10-06T20:54:27Z

storage/pkg/flagset/storagemetadata.go

@@ -73,7 +88,15 @@ func StorageMetadata(cfg *config.Config) []cli.Flag {
 	flags = append(flags, DriverLocalWithConfig(cfg)...)
 	flags = append(flags, DriverOwnCloudWithConfig(cfg)...)
 	flags = append(flags, DriverOCISWithConfig(cfg)...)
-
+	flags = append(flags,


@IljaN we need to append the storage-root to the end, otherwise the evaluation of the config flags for the drivers will overwrite the default that was written into &cfg.Reva.Storages.Common.Root.

butonic · 2020-10-06T20:55:57Z

storage/pkg/flagset/storagepubliclink.go

 		},

 		&cli.StringFlag{
 			Name:        "mount-path",
-			Value:       "/public/",
+			Value:       "/public",


@refs double check if public links work. I also updated the registry to use a path without slash, like the other mountpoints: https://github.com/owncloud/ocis/pull/657/files?file-filters%5B%5D=.go#diff-76cf344e66e9654cf08805e489b5d6c2R230

butonic · 2020-10-06T20:57:58Z

storage/pkg/flagset/users.go

@@ -20,55 +20,48 @@ func UsersWithConfig(cfg *config.Config) []cli.Flag {

 		// Services

-		// Users
+		// Userprovider


I renamed the STORAGE_USERS prefix for the userprovider service to STORAGE_USERPROVIDER, this way I could use STORAGE_USERS for the storage provider mounted at /users

C0rby · 2020-10-07T18:27:10Z

Oh I can push changes to butonic:rename-storage. That's nice.

C0rby · 2020-10-08T16:41:22Z

After renaming the environment variables the eos setup worked.
I'm not sure what I need to do for the ldap setup..

storage/pkg/flagset/frontend.go

storage/pkg/flagset/gateway.go

individual-it · 2020-10-12T10:09:55Z

storage/pkg/flagset/storageoc.go

-			Name:        "data-server-url",
-			Value:       "http://localhost:9164/data",
-			Usage:       "data server url",
-			EnvVars:     []string{"STORAGE_STORAGE_OC_DATA_SERVER_URL"},


used in drone

ocis/.drone.star

Line 559 in d422a91

'STORAGE_STORAGE_OC_DATA_SERVER_URL': 'http://ocis-server:9164/data',

not needed anymore?

Yes, the storages got streamlined so there is no OC storage anymore.

individual-it · 2020-10-12T10:11:04Z

storage/pkg/flagset/storageocdata.go

-			Name:        "temp-folder",
-			Value:       "/var/tmp/",
-			Usage:       "temp folder",
-			EnvVars:     []string{"STORAGE_STORAGE_OC_DATA_TEMP_FOLDER"},


used in drone

ocis/.drone.star

Line 554 in d422a91

'STORAGE_STORAGE_OC_DATA_TEMP_FOLDER': '/srv/app/tmp/',

not needed anymore?

Yes, the storages got streamlined so there is no OC storage anymore.

individual-it · 2020-10-12T10:12:15Z

storage/pkg/flagset/storagehomedata.go

-			Name:        "temp-folder",
-			Value:       "/var/tmp/",
-			Usage:       "temp folder",
-			EnvVars:     []string{"STORAGE_STORAGE_HOME_DATA_TEMP_FOLDER"},


used in drone

ocis/.drone.star

Line 550 in d422a91

'STORAGE_STORAGE_HOME_DATA_TEMP_FOLDER': '/srv/app/tmp/',

not needed anymore?

Yes, this configuration option does not exist anymore.

individual-it · 2020-10-12T10:13:01Z

storage/pkg/flagset/storageoc.go

-			Name:        "driver",
-			Value:       "owncloud",
-			Usage:       "storage driver for oc mount: eg. local, eos, owncloud, ocis or s3",
-			EnvVars:     []string{"STORAGE_STORAGE_OC_DRIVER"},


used in drone

ocis/.drone.star

Line 548 in d422a91

'STORAGE_STORAGE_OC_DRIVER': '%s' % (storage),

not needed anymore?

Yes, the storages got streamlined so there is no OC storage anymore.

individual-it · 2020-10-12T10:13:34Z

storage/pkg/flagset/storageocdata.go

-			Name:        "driver",
-			Value:       "owncloud",
-			Usage:       "storage driver for oc data mount: eg. local, eos, owncloud, ocis or s3",
-			EnvVars:     []string{"STORAGE_STORAGE_OC_DATA_DRIVER"},


used in drone

ocis/.drone.star

Line 549 in d422a91

'STORAGE_STORAGE_OC_DATA_DRIVER': '%s' % (storage),

not needed anymore?

Yes, the storages got streamlined so there is no OC storage anymore.

individual-it · 2020-10-12T10:14:01Z

storage/pkg/flagset/storagehomedata.go

-			Name:        "driver",
-			Value:       "owncloud",
-			Usage:       "storage driver for home data mount: eg. local, eos, owncloud, ocis or s3",
-			EnvVars:     []string{"STORAGE_STORAGE_HOME_DATA_DRIVER"},


used in drone

ocis/.drone.star

Line 547 in d422a91

'STORAGE_STORAGE_HOME_DATA_DRIVER': '%s' % (storage),

not needed anymore?

This option does not exist anymore.

I currently don't know why but locally before renaming the service would always shutdown prematurely Signed-off-by: David Christofas <dchristofas@owncloud.com>

Signed-off-by: David Christofas <dchristofas@owncloud.com>

…id mapping Signed-off-by: David Christofas <dchristofas@owncloud.com>

Signed-off-by: David Christofas <dchristofas@owncloud.com>

C0rby · 2020-10-21T09:50:46Z

Oh ffs... now apiWebdavEtagPropagation1/moveFileFolder.feature:20 is switching between passing and failing...

Signed-off-by: David Christofas <dchristofas@owncloud.com>

C0rby · 2020-10-21T14:16:08Z

Oh I hope it'll work.

accessible through the reva data gateway

sonarcloud · 2020-10-21T14:48:29Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

No Coverage information
No Duplication information

kulmann · 2020-10-21T15:31:13Z

🥳

Merge: d9d7272 8445996 Author: David Christofas <C0rby@users.noreply.github.com> Date: Wed Oct 21 17:22:15 2020 +0200 Merge pull request #657 from butonic/rename-storages update reva and refactor config

butonic commented Oct 6, 2020

View reviewed changes

butonic requested review from micbar, IljaN and refs October 6, 2020 20:58

butonic force-pushed the rename-storages branch from 43668da to 505f737 Compare October 6, 2020 21:28

C0rby force-pushed the rename-storages branch from 14e8c76 to 72b86f0 Compare October 12, 2020 09:28

individual-it reviewed Oct 12, 2020

View reviewed changes

storage/pkg/flagset/frontend.go Show resolved Hide resolved

individual-it reviewed Oct 12, 2020

View reviewed changes

storage/pkg/flagset/gateway.go Show resolved Hide resolved

individual-it reviewed Oct 12, 2020

View reviewed changes

David Christofas added 13 commits October 21, 2020 10:15

rename command

5257e0d

I currently don't know why but locally before renaming the service would always shutdown prematurely Signed-off-by: David Christofas <dchristofas@owncloud.com>

update expected failures

1084d45

Signed-off-by: David Christofas <dchristofas@owncloud.com>

change mount id of home storage

001fb86

Signed-off-by: David Christofas <dchristofas@owncloud.com>

change ocis log level to default and change public link storage mount…

dd4c320

…id mapping Signed-off-by: David Christofas <dchristofas@owncloud.com>

clean up storage configuration and update storage documentation

4e548da

Signed-off-by: David Christofas <dchristofas@owncloud.com>

clean up user data in ocis storage

655c7a1

Signed-off-by: David Christofas <dchristofas@owncloud.com>

update expected failures

4a20f14

Signed-off-by: David Christofas <dchristofas@owncloud.com>

fix OCIS_REVA_DATA_ROOT

a6a6354

Signed-off-by: David Christofas <dchristofas@owncloud.com>

update reva

ee7260c

Signed-off-by: David Christofas <dchristofas@owncloud.com>

update expected failures

f51da17

Signed-off-by: David Christofas <dchristofas@owncloud.com>

fix OCIS_REVA_DATA_ROOT

9ae1891

Signed-off-by: David Christofas <dchristofas@owncloud.com>

update expected failures

916c984

Signed-off-by: David Christofas <dchristofas@owncloud.com>

update reva

ec692d3

Signed-off-by: David Christofas <dchristofas@owncloud.com>

C0rby force-pushed the rename-storages branch 2 times, most recently from 5fe9109 to 3c28fda Compare October 21, 2020 09:30

update expected failures

63eede3

Signed-off-by: David Christofas <dchristofas@owncloud.com>

C0rby force-pushed the rename-storages branch from 3c28fda to 63eede3 Compare October 21, 2020 09:56

kulmann added 2 commits October 21, 2020 16:05

Don't expose data servers

7094375

Bring back data server URLs..........

91977ab

kulmann added 2 commits October 21, 2020 16:32

Bring back exposing user storage

1da23d1

Confirmed that both the home storage and users storage need to be

8445996

accessible through the reva data gateway

butonic changed the title ~~[WIP] update reva and refactor config~~ update reva and refactor config Oct 21, 2020

kulmann approved these changes Oct 21, 2020

View reviewed changes

C0rby merged commit 8aec764 into owncloud:master Oct 21, 2020

butonic deleted the rename-storages branch October 21, 2020 15:33

update reva and refactor config #657

update reva and refactor config #657

Conversation

butonic commented Oct 6, 2020 • edited by C0rby Loading

update-docs bot commented Oct 6, 2020

butonic commented Oct 6, 2020

butonic commented Oct 6, 2020

butonic commented Oct 6, 2020

butonic commented Oct 6, 2020

butonic commented Oct 6, 2020

butonic commented Oct 6, 2020

butonic commented Oct 6, 2020

butonic commented Oct 6, 2020

butonic Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

butonic Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

butonic Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

butonic Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C0rby commented Oct 7, 2020

C0rby commented Oct 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C0rby commented Oct 21, 2020

C0rby commented Oct 21, 2020

sonarcloud bot commented Oct 21, 2020

kulmann commented Oct 21, 2020

butonic commented Oct 6, 2020 •

edited by C0rby

Loading

butonic Oct 6, 2020 •

edited

Loading

butonic Oct 6, 2020 •

edited

Loading

butonic Oct 6, 2020 •

edited

Loading

butonic Oct 6, 2020 •

edited

Loading